Repository navigation
Conversation
Add DebugLayoutReadoutState and internal DebugLayoutReadout composable to display window container size in dp, display rotation degrees, rotation-lock status, and active adaptive layout mode in DebugConsole, falling back to live LocalWindowInfo container dimensions when explicit posture state is omitted. Also fix DebugTextBar applying its outer Row modifier three times to its child Text nodes, configure Robolectric Compose unit testing for :ui:debug, and add unit tests covering explicit/default readouts, live window fallback, modifier scoping, and hidden-component behavior.
…viewDisplay - Extract @VisibleForTesting internal calculateViewfinderSize with non-positive bounds guard and landscape aspect ratio selection when useLandscapeAspectRatio && maxWidth > maxHeight - Add contentAlignment and useLandscapeAspectRatio trailing default parameters to PreviewDisplay - Replace BoxWithConstraints with standard Box + Modifier.layout measurement and graphicsLayer draw-phase blink alpha/clip - Stabilize gesture callbacks with rememberUpdatedState and migrate rememberTransformableState to non-deprecated centroid overload - Enforce LayoutDirection.Ltr inside PreviewDisplay viewfinder scope so tap-to-focus coordinates and FocusMeteringIndicator never mirror in RTL - Add PreviewDisplay @previews and Robolectric unit/Compose tests in PreviewDisplayTest
…yout-readout # Conflicts: # ui/debug/src/main/java/com/google/jetpackcamera/ui/debug/DebugOverlayComponents.kt
…80-degree gravity delta
# Conflicts: # ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureScreenComponents.kt
…dow orientation - Remove static android:screenOrientation="nosensor" from MainActivity in AndroidManifest.xml and declare orientation|screenSize|smallestScreenSize|screenLayout|keyboardHidden configChanges - Add runtime resolveRequestedOrientation in MainActivity to lock full-screen compact phones (< 600dp) to SCREEN_ORIENTATION_NOSENSOR while using SCREEN_ORIENTATION_UNSPECIFIED in multi-window and on large screens (>= 600dp) to prevent 1:1 OS compatibility letterboxing - Pass useLandscapeAspectRatio = true to PreviewDisplay in PreviewScreen so wide containers frame the viewfinder in landscape (4:3 / 16:9) - Add unit tests in MainActivityOrientationTest
…/debug-layout-readout # Conflicts: # feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt # ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureLayout.kt
…remapping, and mode readout
…remapping, and mode readout
…display-framing # Conflicts: # feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt # ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/CaptureScreenComponents.kt
…/debug-layout-readout # Conflicts: # core/model/src/main/java/com/google/jetpackcamera/model/DeviceRotation.kt # feature/preview/src/main/java/com/google/jetpackcamera/feature/preview/PreviewScreen.kt # ui/components/capture/src/main/java/com/google/jetpackcamera/ui/components/capture/DebouncedOrientationFlow.kt
…on, active layout readout, and recording orientation lock
…to kim/adaptive/layout-rotation
…/debug-layout-readout
… and prune dead helper Co-authored-by: davidjiagoogle <[email protected]>
…ug readout Co-authored-by: davidjiagoogle <[email protected]>
…gree animation toggle
…ew in layout-rotation
…snap 180-degree flips
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive layout and rotation overhaul to support seamless, posture-aware layouts across various form factors (compact phones, foldables, and tablets), including dynamic orientation locking during video recording and layout-level counter-rotation. The changes are well-supported by extensive new unit and Robolectric UI tests. The code review feedback correctly identifies several critical backward compatibility issues that would cause NoSuchMethodError crashes on devices running older Android versions (API < 23 and API < 24) due to unguarded platform calls to Context.getSystemService(Class) and Activity.isInMultiWindowMode. Additionally, the feedback points out a Compose side-effect anti-pattern in the debug overlay where state is mutated directly during composition, providing an actionable suggestion to wrap the update in a LaunchedEffect.
| val surfaceRotation = context.getSystemService(DisplayManager::class.java) | ||
| ?.getDisplay(Display.DEFAULT_DISPLAY) | ||
| ?.rotation |
There was a problem hiding this comment.
Using context.getSystemService(DisplayManager::class.java) directly will crash with a NoSuchMethodError on devices running API levels below 23 (Android 6.0), as Context.getSystemService(Class) was introduced in API 23. Since the Jetpack Camera App supports API 21+, please use context.getSystemService(Context.DISPLAY_SERVICE) as? DisplayManager to ensure backward compatibility.
val displayManager = context.getSystemService(Context.DISPLAY_SERVICE) as? DisplayManager\n val surfaceRotation = displayManager\n ?.getDisplay(Display.DEFAULT_DISPLAY)\n ?.rotation|
|
||
| private fun updateRequestedOrientation( | ||
| configuration: Configuration, | ||
| multiWindowMode: Boolean = isInMultiWindowMode |
There was a problem hiding this comment.
Accessing isInMultiWindowMode as a default parameter value will cause a NoSuchMethodError crash on startup on devices running API levels below 24 (Android 7.0), as Activity.isInMultiWindowMode was introduced in API 24. Please guard this platform call with an API level check.
multiWindowMode: Boolean = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {\n isInMultiWindowMode\n } else {\n false\n }| val activity = LocalActivity.current ?: remember(context) { context.findDebugActivity() } | ||
|
|
||
| var isInMultiWindowMode by remember(activity) { | ||
| mutableStateOf(activity?.isInMultiWindowMode ?: false) |
There was a problem hiding this comment.
Calling activity?.isInMultiWindowMode directly will cause a NoSuchMethodError crash on devices running API levels below 24 (Android 7.0). Please guard this platform call with an API level check to ensure backward compatibility on API 21-23 devices.
mutableStateOf(\n if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {\n activity?.isInMultiWindowMode == true\n } else {\n false\n }\n )| mutableStateOf(activity?.isInMultiWindowMode ?: false) | ||
| } | ||
| DisposableEffect(activity) { | ||
| isInMultiWindowMode = activity?.isInMultiWindowMode ?: false |
There was a problem hiding this comment.
Calling activity?.isInMultiWindowMode directly inside DisposableEffect will cause a NoSuchMethodError crash on devices running API levels below 24 (Android 7.0). Please guard this platform call with an API level check.
isInMultiWindowMode = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) {\n activity?.isInMultiWindowMode == true\n } else {\n false\n }| var cumulativeTargetDegrees by remember { mutableFloatStateOf(targetAngle) } | ||
| val currentNormalized = ((cumulativeTargetDegrees % 360f) + 360f) % 360f | ||
| var delta = targetAngle - currentNormalized | ||
| if (delta > 180f) delta -= 360f | ||
| if (delta <= -180f) delta += 360f | ||
| if (delta != 0f) { | ||
| cumulativeTargetDegrees += delta | ||
| } |
There was a problem hiding this comment.
Updating cumulativeTargetDegrees directly in the composable body during composition is a side-effect anti-pattern in Jetpack Compose. It can cause unnecessary recompositions or inconsistent state. Please wrap this state update in a LaunchedEffect keyed on targetAngle.
var cumulativeTargetDegrees by remember { mutableFloatStateOf(targetAngle) }\n LaunchedEffect(targetAngle) {\n val currentNormalized = ((cumulativeTargetDegrees % 360f) + 360f) % 360f\n var delta = targetAngle - currentNormalized\n if (delta > 180f) delta -= 360f\n if (delta <= -180f) delta += 360f\n if (delta != 0f) {\n cumulativeTargetDegrees += delta\n }\n }
Summary
This PR joins the adaptive layout orchestrator track (
kim/adaptive/layout-orchestrator) with the rotation and debug readout track (kim/adaptive/debug-layout-readout). Previously, horizontal<600dpor<480dp-tall compact windows (COMPACT/COMPACT_SLIM, such as800×400dphorizontal split-screen panes or740×360dplandscape phone windows) routed toCompactPortraitLayoutwithout counter-rotating, stacking top bar, viewfinder, and bottom controls vertically inside320–400dpof height. This change counter-rotates the compact UI chrome into portrait along the window's long axis while hosting the live viewfinder in an un-rotated outer layer (OuterViewfinderHost) soSurfaceView(ImplementationMode.EXTERNAL) and 10-bit HDR remain active across all postures. It also adds physical window-inset and display-cutout coordinate remapping, seamless rotation transitions, and liveCameraLayoutMode/LocalActiveCaptureLayouttelemetry inDebugLayoutReadout.Key Changes
RotationModifiers.kt):Modifier.counterRotateToPortrait(windowRotation)to swap width/heightConstraintsand counter-rotate landscape (Rotated90/Rotated270) subtrees to canonical portrait (0°) viaplaceWithLayer, acting as a strict no-op forNaturalandRotated180.PortraitWindowInsets/WindowInsets.toPortraitWindowInsets(windowRotation)to remap physical OS window edges into portrait coordinates independently ofLayoutDirection.LocalCompactCounterRotation(-90,0, or+90) andSeamlessRotationAnimationEffectrequestingROTATION_ANIMATION_SEAMLESSon API 26+ whileCompactPortraitLayoutis active and restoring the prior window attribute on disposal.OuterViewfinderHost, & Inset Wiring (CompactPortraitLayout.kt,CaptureScreenComponents.kt,QuickSettingsScreen.kt):viewfinderinOuterViewfinderHost(un-rotated host coordinate frame, outsideBottomSheetScaffold(containerColor = Color.Transparent)) paired with a same-passExternalViewfinderProxy(ExternalViewfinderMetrics) inside the counter-rotated portrait chrome and sharedModifier.suppressBackgroundForModal(isModalOpen).solvedBandplacement inPreviewDisplayso4:3/16:9frames align cleanly between the counter-rotated top bar and capture controls under bothRotated270(left top bar) andRotated90(right top bar), keepingImplementationMode.EXTERNAL(SurfaceView) on all API > 24 devices.CompactPortraitLayout'sBottomSheetScaffoldandscreenFlashOverlayin.counterRotateToPortrait(effectiveWindowRotation)wheneverwindowWidth > windowHeight, deriving re-anchoredLocalDeviceRotation,LocalWindowRotation, andLocalGravityRotationStatevalues for descendants, and applied.toPortraitWindowInsets(...)across status bar, cutout, navigation bar, and gesture insets.CutoutAwareRow.kt):cutoutInsets.getBottom(this)during measurement and updatedresolveLocalCutoutRectsto transform OS windowDisplayCutoutrects into local portrait LTR/RTL coordinates across all fourDeviceRotationstates (withview.width/view.heightpass-1 fallbacks and landscape RTL span selection).CaptureLayout.kt,CaptureScaffold.kt,SideRailCaptureLayouts.kt,PreviewScreen.kt):deviceRotationandwindowRotationthroughCaptureScaffoldandCaptureScaffoldLayoutintoCompactPortraitLayout, and enableduseLandscapeAspectRatio = trueonPreviewDisplay.LocalWindowRotation = DeviceRotation.NaturalandLocalGravityRotationState = DefaultGravityRotationStateinSideRailCaptureScaffoldsoFoldableLayout,TabletPortraitLayout,TabletLandscapeLayout, andCompactShortLayoutnever counter-rotate.LocalCameraLayoutMode.current.nameandLocalActiveCaptureLayout.currentintoDebugLayoutReadoutState(replacing the pre-join"CaptureScaffoldLayout"placeholder).Technical Notes
windowWidth > windowHeight): Counter-rotation is driven by container aspect rather than rawwindowRotation.isLandscape. A tall side-by-side split-screen pane on a rotated phone (380×760dpatRotated90) stays unrotated (effectiveWindowRotation = Natural), whereas a wide horizontal split-screen pane on an upright device (800×400dpor550×500dpatNatural) counter-rotates-90°CCW (Rotated270, placing TopBar on the left and Shutter on the right) and setscontainerFrameRotation = Rotated90soLocalGravityRotationStatesupplies+90°CW to keep child icons upright.OuterViewfinderHostsits outside both.counterRotateToPortraitandBottomSheetScaffold:SurfaceView(ImplementationMode.EXTERNAL, required for 10-bitCOLOR_MODE_HDR) punches through viaSurfaceControland ignoresRenderNoderotationZtransforms; keepingOuterViewfinderHostun-rotated lets CameraX'sSurfaceRequestorient frames directly on a hardwareSurfaceView.BottomSheetScaffoldsubcomposes its body viaSubcomposeLayoutduring layout measurement; keepingmovableViewfinderin a standardLayoutoutsideBottomSheetScaffoldkeeps it in the primary composition pass across all 5 posture layouts, preventingDetectWindowColorModeChangesonCompletionfrom firingCOLOR_MODE_DEFAULTduringCOMPACT <-> TABLET_LANDSCAPEtransitions.w_c × h_cchild at((h_c - w_c) / 2, (w_c - h_c) / 2)inside ah_c × w_clayout alignsTransformOrigin.Centerwith(layoutWidth / 2, layoutHeight / 2), preserving pixel-exact Compose pointer input and semanticsboundsInRootafter±90°layer rotation.Not Included Yet
kim/adaptive/viewfinder-rotation): WiringTransientSessionSettings.windowRotationto updatePreview.targetRotationin place, frozen while recording.180°Retention (kim/adaptive/content-gravity): Applying.animateToUpright()/.rotateLayoutToGravity()across capture buttons,ZoomBarlabels,ToggleSwitchicons,LandscapeTopEdgeBanner, Quick Settings landscape scroll/cross-fade, andCameraErrorDialog— plus sticky180°gravity retention.#649), vertical capture mode switch (#635),QuickSettingsOverlayCardcontainer (#648),Tooltippolish (#622), andZoomBarRTL polish (#644) remain unchanged fromkim/adaptive/layout-orchestrator.Testing
LayoutRotationTest(new): Robolectric Compose UI tests covering full-screen740×360dplandscape phone counter-rotation (Rotated90&Rotated270) with uprightOuterViewfinderHost,380×760dpportrait split-screen non-rotation on a rotated phone,3 × 3horizontal split-screen matrix (852×420dp,800×400dp,550×500dp×Natural/Rotated90/Rotated270), non-rotation acrossFOLDABLE/TABLET_PORTRAIT/TABLET_LANDSCAPE/COMPACT_SHORT,CutoutAwareRowLTR/RTL cutout pill clearance, and uninterrupted 10-bitHLG_10_BITCOLOR_MODE_HDRacross412×915dp -> 800×400dp -> 1280×800dp -> 412×915dp.PreviewDisplayTest: Verified horizontal solved-band alignment underLocalCompactCounterRotation.RotationModifiersTest: Unit/Compose tests forcounterRotateToPortrait(Natural,Rotated180,Rotated90,Rotated270, and unbounded width safety) andtoPortraitWindowInsetsacross all 4 rotations in LTR and RTL.CutoutAwareRowTest: Added pass-1 (rowBoundsInWindow == null)Rotated90/Rotated270cutout remapping assertions usingfallbackViewWidthandfallbackViewHeight.DebugLayoutReadoutTest: VerifiedDEBUG_LAYOUT_MODE_TAGrenderscameraLayoutModewhen populated byCaptureScaffoldLayout.